Skip to content

debugfs.rs: Rust API for debugfs#702

Closed
adamrk wants to merge 2 commits intoRust-for-Linux:rustfrom
adamrk:debugfs
Closed

debugfs.rs: Rust API for debugfs#702
adamrk wants to merge 2 commits intoRust-for-Linux:rustfrom
adamrk:debugfs

Conversation

@adamrk
Copy link
Copy Markdown

@adamrk adamrk commented Mar 11, 2022

Add a Rust API to create directories and files in debugfs.

Signed-off-by: Adam Bratschi-Kaye ark.email@gmail.com

Comment thread rust/kernel/debugfs.rs Outdated
@adamrk adamrk force-pushed the debugfs branch 2 times, most recently from 7d75b10 to 0400797 Compare March 12, 2022 20:20
Comment thread rust/kernel/debugfs.rs Outdated
/// Create a new directory in `debugfs` under `parent`. The directory will
/// be removed when the parent is dropped.
pub fn create_with_parent<'name, 'parent>(
name: &'name CStr,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if 'name is only used here, wouldn't it be cleaner to elide it?

Comment thread rust/kernel/debugfs.rs
) -> Result<DebugFsFile<T>> {
let name = name.as_char_ptr();
let data = data.into_pointer() as *mut _;
let parent = parent.map(|p| p.dentry).unwrap_or_else(ptr::null_mut);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why you can't use this same sequence (map/unwrap_or_else) to get the parent pointer when creating directories above?

Comment thread rust/kernel/debugfs.rs Outdated
}
// SAFETY: `self.data` was created by a call to `T::into_pointer` in
// [`DebugFsFile::create`].
unsafe { T::from_pointer(self.data) };
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens in the windows between this happening and the parent being dropped?

It seems like we have a dangling pointer in i_private that could, if accessed, lead to UB. Is there anything preventing this?

Comment thread rust/kernel/debugfs.rs Outdated
/// An `inode` for a file in debugfs with a `T` stored in `i_private`.
pub struct DebugFsFile<T: PointerWrapper> {
dentry: *mut bindings::dentry,
data: *mut c_types::c_void,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the data is stored in the dentry's inode, why do we need to hold a pointer to it here?

Comment thread rust/kernel/debugfs.rs Outdated
dentry: *mut bindings::dentry,
has_parent: bool,
file_children: Vec<Box<dyn Sync>>,
dir_children: Vec<DebugFsDirectory>,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Creating a hierarchy that is recursively dropped in dangerous as it may lead to overflowing the call stack. Is there anything [that I'm failing to see] that prevents recursive calls to drop overflowing the stack? It seems like we need a strategy to clean these up without using recursion to traverse all the elements.

Comment thread rust/kernel/debugfs.rs Outdated
pub struct DebugFsDirectory {
dentry: *mut bindings::dentry,
has_parent: bool,
file_children: Vec<Box<dyn Sync>>,
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there anything we can do with to_raw_parts that would obviate the need to box the file entries?

We already depend on ptr_metadata.

@adamrk adamrk force-pushed the debugfs branch 2 times, most recently from aa79325 to d12ec6f Compare August 6, 2022 13:55
@adamrk adamrk force-pushed the debugfs branch 2 times, most recently from 9d8bad2 to 795d87b Compare September 6, 2022 13:55
Add a Rust API to create directories and files in `debugfs`.

Signed-off-by: Adam Bratschi-Kaye <ark.email@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants